I think that issue is because the flake uses a pinned helix repo for working around submodules. The pinned repo gets outdated and the language submodule isnt available. Since the fix is merged into Nix now, we should remove the workaround once it lands on unstable Nix.

it landed a couple of days ago I think

❯ nix --version
nix (Nix) 2.4pre20210908_3c56f62

(that is the latest version)

I wasn't able to build the project without that fix (where submodules are automatically updated).

thanks for the quick feedback.

Let me know also if this PR looks ok to you.

Ah, if it landed, we should remove the submodule hack (maybe in this PR?).

About the dependency, if libiconv is required on Darwin to build helix, it should be added to the crate overrides' buildInputs for the helix crate that needs it.

so libiconv is already added as a global dependency for darwin in nixpkgs (basically for every crate as a buildInput).
However this PR here aims at the dev workflow, so helping to run cargo build. That will fail with a missing libiconv. This is why I added it to the devDependencies. It's only required for compilation on dev not with nix.

Let me know if that makes sense or if I misunderstood something.

I've removed the helix version fix.
I think i'm still missing something though, I'm not getting syntax highlighting on my files even with the built version.

so libiconv is already added as a global dependency for darwin in nixpkgs (basically for every crate as a buildInput).
However this PR here aims at the dev workflow, so helping to run cargo build. That will fail with a missing libiconv. This is why I added it to the devDependencies. It's only required for compilation on dev not with nix.

Let me know if that makes sense or if I misunderstood something.

Ah I see, yeah it makes sense to put it in dev only. I didn't know that some deps were "hardcoded" in buildRustCrate instead of using defaultCrateOverrides. I pushed a commit to nix-cargo-integration master that should add libiconv to devshell when on darwin, so you should update that and see if it does the job.

Just this one https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/rust/default.nix#L125
it is needed in a lot of crates, so rather than adding to 20 crates, somebody suggested just adding it globally.

What do you think about the commit that removes the fixed helix dependency ?

let me test the latest nix-cargo-integration.

I tested with updated flake inputs and the dev shell is working perfectly.

I removed the commit adding libiconv to the dev dependencies.

Let me know about the version fix removal thing.

Hmm, the syntax highlighting is still not working? I guess it might be something else then? Could you also check and see if Protobuf files are highlighted? They also do not highlight for me so I'm wondering if it's the same problem.

It might not be picking up new submodules? I had to git submodule update after a pull to get any new grammars that are added.

I just checked on a protobuf file with the change included in this PR and yes syntax highlighting is working for a message.proto containing the following

message SearchRequest {
  required string query = 1;
  optional int32 page_number = 2;
  optional int32 result_per_page = 3;
}

@archseer you might be right here, it could be a change not picked up in the submodule. I'm going to read more about submodules and nix.

I think this PR can be merged though.

@yusdacra just a littlle more information on this.
The PR reference enabling submodules for flakes, enables them for self, i.e. flakes that you have downloaded and you are working on.
The syntax still doesn't work for github:owner/repo. Meaning that if you need a particular commit, you need to download the flake locally and refer it by file path. I don't think it's a particular problem personally.
There is work ongoing to make the github:owner/repo syntax. It might take another 3 months before it actually lands.
I was thinking of just adding something in the readme to say that the flake needs to be downloaded locally. Let me know what you think.